feat(fonts): resolve fonts per document, not globally#3626
Open
caio-pizzol wants to merge 10 commits into
Open
feat(fonts): resolve fonts per document, not globally#3626caio-pizzol wants to merge 10 commits into
caio-pizzol wants to merge 10 commits into
Conversation
Replace the global frozen logical->physical map with a FontResolver INSTANCE so two editors on one page can map the same logical family differently without leaking - the same per-document isolation the registry already has per FontFaceSet. Each instance is seeded with the five bundled clean clones and holds per-instance runtime overrides (map/unmap) with a version epoch for reuse-busting. Behavior-preserving: the module-level resolve* functions delegate to a shared default instance, so every current caller (measure, paint, planner, gate, report) is unchanged. Threading the per-document instance through those call sites is the next step.
…and report PresentationEditor now owns one FontResolver per document and passes it to the planner (planRequiredFontFaces), the gate (which derives the family-path resolution from it and resolves its report through it), and buildFontReport. So load, family resolution, and diagnostics all go through the same per-document instance instead of the global default. Behavior-preserving: with no runtime overrides the resolver matches the bundled map, and the threaded params are optional (callers without a document context still use the global functions). Measure and paint still use the global resolver + cache keys; threading those (with the resolver signature in the measure cache key and paint reuse signature) is next.
…rsion docs - Add FontResolver.reset() and call it from documentReplaced alongside the gate reset. The resolver is per PresentationEditor instance, so a reused editor must drop the prior document's runtime mappings or a fonts.map would leak into the next document. - Fix the class JSDoc: the cache/reuse identity is the stable signature, not the numeric version (two docs at the same version with different mappings must not collide).
Measure and paint still use the global resolver; only planner, gate, and report read the per-document instance so far. Reword the #fontResolver field and the gate fontResolver option so they do not claim measure/paint consistency before that threading lands.
…tring) measureBlock now accepts a resolvePhysical function and threads it through measureParagraphBlock / measureListBlock / measureTableBlock / measureTabAlignmentGroup / measureDropCap to buildFontString - including the table path's recursive measureBlock call for cell content - so a document's text is MEASURED with its own resolver (honoring a per-document fonts.map) instead of the global one. Defaults to the global bundled map, so callers that pass nothing are unchanged; the body/header-footer callers that bind the document resolver, plus the measure-cache font signature, come next. Verified: measuring-dom tsc error profile is identical to baseline (no new errors vs the pre-existing implicit-any/never backlog).
Thread the document FontResolver and its mapping signature through every measure path - body, footnotes, header/footer, and per-rId header/footer - so two editors that map the same logical family differently cannot share a cached measure. The signature rides a narrow incremental-layout runtime option (not the exported LayoutOptions) and keys each measure cache; the resolver itself rides the measure callback. previousMeasures reuse bypasses the cache key, so it is now gated on the signature matching the prior pass. PresentationEditor binds its resolver into the measure callback and records the signature its measures were produced with. Behavior-preserving: no public fonts.map yet, so the signature stays '' and every path resolves through the bundled map as before. Paint is next.
The painter and the paint-reuse versioning now resolve fonts through the document's per-instance FontResolver instead of the global one, completing the per-document isolation the measure path already has. The painter is per document, so its resolvePhysical comes from painter options (PresentationEditor binds its resolver); text runs paint the family it returns, the same one measurement used. The resolver's signature is folded into every block's paint-reuse version - body via resolveLayout, header/footer via the session - so a future fonts.map change repaints the way a font load does today. Behavior-preserving: no public fonts.map yet, so the signature stays '' and both paint output and reuse versions are byte-identical to before.
Two documents that map one logical family to different physical fonts must not share a cached measure or reuse each other's painted DOM. The MeasureCache test keys two signatures against one block; the resolveLayout test drives two real resolvers (Georgia->Gelasio vs Georgia->Tinos) and asserts their block paint-reuse versions differ. Both also assert the empty signature is byte-identical to omitting it, locking in the behavior-preserving default.
…hreading Two comments still said measure and paint do not read the per-document resolver - true when written in the planner/gate phase, false now that both resolve through it (a stale comment is a prompt-surface bug). Also soften two "byte-identical to the prior global behavior" claims to "behavior-preserving by construction": the signature is always '' and the resolver shares the bundled map, so resolution is unchanged, though that is not golden-tested.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c253836e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…path The resolver docs claimed measure and paint all resolve through the per-document instance. That over-claims: field-annotation pills measure (line-layout path) and paint with the logical family - pre-existing on main, unchanged here. Tighten the #fontResolver and gate JSDocs to scope the claim to text runs and name the pill exception, deferring the fix to the fonts.map PR (where unifying it is an intended rendering change).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Give each document its own font resolver instead of one global table. Today every logical font resolves to its physical substitute (Calibri to Carlito) through a shared module, so two editors on a page can't map the same family differently, and a future per-document
fonts.mapwould leak across documents.Each document now owns a resolver, threaded through planner, gate, report, measure, and paint, with a stable mapping signature folded into every measure cache and paint-reuse version so two documents can't share a measure or reuse each other's painted DOM. The platform-neutral layout packages take a signature string and a resolve function, never the resolver class - the resolver lives only in the v1 editor.
No user-facing change: there's no public mapping API yet, so every signature is empty and the resolver shares the same bundled substitute map, behavior-preserving by construction. This is the foundation the write API (
fonts.map/add/preload) builds on next.One known exception, called out in review: field-annotation pills (forms/content controls) still measure and paint with the logical family, exactly as on
main- this PR does not touch that path. Making them resolver-aware changes their rendering, so it lands with thefonts.mapPR, not this behavior-preserving foundation.Verified: per-package
tsc --noEmitvs the pre-existing baseline -> no new errors; painter boundary check -> no forbidden imports. CI runs the unit tests.Review: the signature is cache identity, not the numeric version (two docs at version 1 with different mappings must not collide);
previousMeasuresreuse bypasses the cache key, so it's gated on the signature separately.